Skip to content

London | 26-ITP-Sept | Fung Nin Lee | Sprint 2 | Implement and rewrite test - #1678

Open
leerogerfn wants to merge 15 commits into
CodeYourFuture:mainfrom
leerogerfn:coursework/sprint-2
Open

leerogerfn wants to merge 15 commits into
CodeYourFuture:mainfrom
leerogerfn:coursework/sprint-2

Conversation

@leerogerfn

Copy link
Copy Markdown

Learners, PR Template

Self checklist

  • I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
  • My changes meet the requirements of the task
  • I have tested my changes
  • My changes follow the style guide

Task code

CYF-1059

Changelist

Completed writing functions, node test and jest test

Questions

:)

@github-actions

This comment has been minimized.

@leerogerfn leerogerfn changed the title London | 26-ITP-Sept | Fung Nin Lee | Sprint-2 | Implement-and-rewrite-tests London | 26-ITP-Sept | Fung Nin Lee | Sprint-2 | Implement and rewrite test Oct 11, 2026
@github-actions

This comment has been minimized.

@leerogerfn leerogerfn added 📅 Sprint 2 Assigned during Sprint 2 of this module Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Oct 11, 2026
@github-actions

This comment has been minimized.

@github-actions github-actions Bot removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Oct 11, 2026
@github-actions

This comment has been minimized.

@leerogerfn leerogerfn changed the title London | 26-ITP-Sept | Fung Nin Lee | Sprint-2 | Implement and rewrite test London | 26-ITP-Sept | Fung Nin Lee | Sprint 2 | Implement and rewrite test Oct 11, 2026
@leerogerfn leerogerfn added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Oct 13, 2026
@hackertainment hackertainment added Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Oct 15, 2026

@hackertainment hackertainment left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your coursework. Please try to consider increasing test cases coverage to include more possible inputs. Keep it up.

Comment on lines +28 to 31
test("Classifies boundary and invalid angles", () => {
assert.equal(getAngleType(365), "Invalid angle");
assert.equal(getAngleType(-10), "Invalid angle");
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The two cases included here are general cases for invalid angles. There are two boundary cases as well. Can you think of them? If so, please add them as well. Thank you.

Comment on lines 36 to +40
// Case 6: Invalid angles
test(`should return "Invalid angle" when (angel > 360 || angle < 1)`, () => {
expect(getAngleType(-10)).toEqual("Invalid angle");
expect(getAngleType(365)).toEqual("Invalid angle");
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The two cases included here are general cases for invalid angles. There are two boundary cases as well. Can you think of them? If so, please add them as well. Thank you.

Comment on lines +17 to +20
if (numerator < denominator) {
return "true";
}
return "false";

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the provided example test case, the return type should be boolean rather than string. However, I will let you pass this time - because you will learn more about data type in the future, or if you wish, try to book a mentored coding session with a volunteer to clear the concept of data type as early as possible, Thank you.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about numerator the same as denominator? how about zero in numerator or denominator? Should they return true or false? Please add them as well. Thank you.

});

test(`should true when denominator and numerator are negative`, () => {
expect(isProperFraction(-5, -2)).toEqual("true");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this one should return false.

);
});

// TODO: What other invalid card cases can you think of?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should be more test cases for invalid card, how about a card with correct number/face without suit? how about a card with correct suit but no number/face? how about a card missing both number/face and suite? how about a card with suit followed by a number/face? would 11♠ be valid or not? Please add more invalid card cases. Thank you.

Comment on lines 21 to +26
// Invalid Cards
test(`Expected a number followed by a suit, but got invalid`, () => {
expect(() => getCardValue("invalid")).toThrow(
`Expected a number followed by a suit, but got "invalid"`
);
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as my another comment https://github.com/CodeYourFuture/Module-Structuring-and-Testing-Data/pull/1678/changes#r4282076991 , please add more invalid card cases. Thank you.

@hackertainment hackertainment added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. labels Oct 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Reviewed Volunteer to add when completing a review with trainee action still to take. 📅 Sprint 2 Assigned during Sprint 2 of this module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants